West Midlands | 26-Jan-ITP | Fida H Ali Zada | Sprint 2 | coursework/sprint-2#946
West Midlands | 26-Jan-ITP | Fida H Ali Zada | Sprint 2 | coursework/sprint-2#946alizada-dev wants to merge 12 commits intoCodeYourFuture:mainfrom
Conversation
| function calculateBMI(weight, height) { | ||
| // return the BMI of someone based off their weight and height | ||
| } No newline at end of file | ||
| let bmi = weight / (height * height); |
There was a problem hiding this comment.
I noticed you've used let here and in most other places. Could we use another keyword which might be a better fit?
There was a problem hiding this comment.
I used const instead of let
There was a problem hiding this comment.
Nice! 👌
Could you also review the other places where let is being used?
For reference, here is the CYF Code Style Guide.
There was a problem hiding this comment.
Yes, I reviewed the other exercises and changed let to const here as well: const value = Number(penceString.slice(0, -1)) / 100; which was required.
There was a problem hiding this comment.
Nice! 👍 I can still see a few places where we can use const. Could you find them?
There was a problem hiding this comment.
Yup! 👍
Could you review this file too?
There was a problem hiding this comment.
I believe const is not suitable here:
let hours = Number(time.slice(0, 2));
let minutes = Number(time.slice(3, 5));
let ampm = hours >= 12 ? 'pm' : 'am';
because the variable value changes in the next lines of code. I changed these to const but gave error.
There was a problem hiding this comment.
I believe one of them can be const because it is not reassigned. Could you work out which one?
There was a problem hiding this comment.
Ahh, you got me there!! Thanks for pointing it out. Changed let to const in the ampm variable.
There was a problem hiding this comment.
Nice one! I've changed the label to complete now.
Learners, PR Template
Self checklist
Changelist
Completed the Sprint-2 coursework exercises of the Structuring and Testing Data module.